Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix character spacing in fonts using ligatures #1325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phipla
Copy link

@phipla phipla commented Oct 14, 2022

What?

Fixes glyph spacing being wrong when fonts use ligatures. This problem was mentioned in issue #440, #871, #1010.

Why?

The suggested workaround was to disable font ligatures, but ligatures are a desirable feature of many fonts, especially on some non-latin scripts, or on fonts that simulate or are inspired by cursive handwriting.

How?

The problem is in PDF-LIB: the code gets the glyph metric this way:

  • get all codepoints
  • for each codepoint, get the glyph for this codepoint
  • extract the metrics for this glyph.

When fonts have ligatures, there might be several glyphs for a single code point, therefore the glyph metric code does not work and misses some glyphs.

This fix just gets the metrics for all glyphs in the font and does not use glyphForCodePoint

Testing?

The problem is pretty easy to spot on the rendered PDF when using a font using ligatures, like Lobster. Integration test 1, which tests font rendering, was used to check that the issue was fixed.

New Dependencies?

None

Screenshots

Before

Screenshot 2022-10-14 at 12 03 01

After

Screenshot 2022-10-14 at 12 04 14

Suggested Reading?

Yes

Anything Else?

No

Checklist

  • I read CONTRIBUTING.md.
  • I read MAINTAINERSHIP.md#pull-requests.
  • I added/updated unit tests for my changes.
  • I added/updated integration tests for my changes.
  • I ran the integration tests.
  • I tested my changes in Node, Deno, and the browser.
  • I viewed documents produced with my changes in Adobe Acrobat, Foxit Reader, Firefox, and Chrome.
  • I added/updated doc comments for any new/modified public APIs.
  • My changes work for both new and existing PDF files.
  • I ran the linter on my changes.

@phipla phipla mentioned this pull request Oct 16, 2022
2 tasks
@handihow
Copy link

Please merge this PR.

@pratik2575
Copy link

Same issue happened in Gujrati & Hindi Fonts.

duncanwerner added a commit to sdllc/pdf-lib that referenced this pull request Aug 20, 2023
the font embedder was dropping alternate glyphs, which broke (in our
case) tnum rendering. fixed via someone else's PR:

Hopding#1325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants